Feat tip toast queue and staking invariants#375
Open
ExcelDsigN-tech wants to merge 2 commits intoOlufunbiIK:mainfrom
Open
Feat tip toast queue and staking invariants#375ExcelDsigN-tech wants to merge 2 commits intoOlufunbiIK:mainfrom
ExcelDsigN-tech wants to merge 2 commits intoOlufunbiIK:mainfrom
Conversation
|
Someone is attempting to deploy a commit to the olufunbiik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds a priority-queued tip toast system for the frontend that deduplicates events and processes high-priority items first, while simultaneously strengthening the staking contract's arithmetic precision by replacing saturating operations with checked arithmetic that propagates overflow errors throughout reward accrual, stake accounting, and slashing logic. Changes
Sequence Diagram(s)sequenceDiagram
participant WS as WebSocket
participant Hook as useNotifications
participant Queue as tipToastQueue
participant NC as NotificationCenter
participant Toast as Toast Component
WS->>Hook: tipReceived event
activate Hook
alt ID in seenNotificationIdsRef
Hook-->>WS: ignore (duplicate)
else
Hook->>Hook: add ID to seenNotificationIdsRef
Hook->>Queue: enqueueTipToast(id, title, message, priority)
activate Queue
alt tipId not seen before
Queue->>Queue: add tipId to seen set
alt priority='high' && active.priority='normal'
Queue->>Queue: demote active to queue
Queue->>Queue: set new item as active
else
Queue->>Queue: append to queue
end
Queue-->>Hook: return true
else
Queue-->>Hook: return false (duplicate)
end
deactivate Queue
Hook->>Hook: update notification state
end
deactivate Hook
Hook->>NC: render via useTipToastQueue
activate NC
NC->>NC: read activeTipToast
NC->>Toast: render with id, priority, title, message
activate Toast
Toast->>Toast: apply styling based on priority
Toast-->>NC: rendered
deactivate Toast
User->>Toast: dismiss/timeout
Toast->>NC: call dismissTipToast(id)
NC->>Queue: dismissTipToast(id)
activate Queue
Queue->>Queue: clear active, advance queue
Queue-->>NC: notify subscribers
deactivate Queue
deactivate NC
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/contracts/staking/src/lib.rs (1)
110-149:⚠️ Potential issue | 🔴 CriticalReject non-positive
unstakeamounts.
amountis never validated. Withamount < 0, Line 127 and Line 149 both subtract a negative number, so the stake andTotalStakedgrow without any inbound transfer, and Line 143 can push the queued withdrawal negative.amount == 0also resets cooldown state for a no-op request.Minimal guard
pub enum Error { Unauthorized = 1, BelowMinimum = 2, NoStake = 3, InsufficientStake = 4, CooldownNotMet = 5, NoUnstakeRequest = 6, AccountSlashed = 7, TransferFailed = 8, NotInitialised = 9, AlreadyInitialised = 10, Overflow = 11, + InvalidAmount = 12, }pub fn unstake(env: Env, artist: Address, amount: i128) -> Result<(), Error> { artist.require_auth(); Self::assert_initialised(&env)?; + if amount <= 0 { + return Err(Error::InvalidAmount); + } let mut info: StakeInfo = env.storage().persistent()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/contracts/staking/src/lib.rs` around lines 110 - 149, The unstake function does not validate the input amount, allowing negative or zero values to incorrectly increase balances and reset cooldowns; add a guard at the start of pub fn unstake(env: Env, artist: Address, amount: i128) to require amount > 0 (return Err(Error::InvalidAmount) or an appropriate error) before artist.require_auth() or immediately after it, so downstream mutations to StakeInfo.amount, TotalStaked, and UnstakeRequest (DataKey::Unstake) cannot be performed with non-positive amounts and all subsequent checked_add/checked_sub operations remain safe (affecting StakeInfo, TotalStaked, and UnstakeRequest.amount, and use of COOLDOWN_LEDGERS).
🧹 Nitpick comments (1)
contracts/contracts/staking/src/test.rs (1)
570-593: Assert the actual sum-of-balances invariant here.This only checks
total_stakeddeltas aroundartist1. Ifslash()corruptedartist2or leftTotalStakedout of sync with stored stakes, the test still passes. Addartist1.amount + artist2.amount == total_staked()before and after slash/restore.Suggested invariant assertions
let total_before_slash = c.total_staked(); + let before_sum = + c.get_stake(&t.artist1).unwrap().amount + c.get_stake(&t.artist2).unwrap().amount; + assert_eq!(before_sum, total_before_slash); let slash_amount = c.slash(&t.artist1); let total_after_slash = c.total_staked(); + let after_sum = + c.get_stake(&t.artist1).unwrap().amount + c.get_stake(&t.artist2).unwrap().amount; + assert_eq!(after_sum, total_after_slash); assert_eq!(total_after_slash, total_before_slash - slash_amount); @@ c.restore(&t.artist1); assert!(!c.is_slashed(&t.artist1)); c.stake(&t.artist1, &MIN_STAKE); - assert_eq!(c.total_staked(), total_after_slash + MIN_STAKE); + let final_total = c.total_staked(); + let final_sum = + c.get_stake(&t.artist1).unwrap().amount + c.get_stake(&t.artist2).unwrap().amount; + assert_eq!(final_total, total_after_slash + MIN_STAKE); + assert_eq!(final_sum, final_total);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/contracts/staking/src/test.rs` around lines 570 - 593, The test test_slash_restore_invariants_do_not_corrupt_totals should assert the sum-of-balances invariant: compute the per-account stakes (via c.get_stake(...) or c.get_stake(&t.artistX).amount) for artist1 and artist2 and assert artist1.amount + artist2.amount == c.total_staked() before calling c.slash(&t.artist1), immediately after the slash (using the observed slash_amount), and again after c.restore(&t.artist1) and the final c.stake(&t.artist1, &MIN_STAKE) to ensure slash/restore didn't corrupt artist2 or TotalStaked; update the assertions in that test around the slash/restore points accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/contracts/staking/src/lib.rs`:
- Around line 285-291: pending_rewards currently uses calculate_accrued (which
hides overflow) and only clamps the final addition; change it to call
calculate_accrued_checked and if that returns Err (overflow) return i128::MAX,
otherwise perform pending =
i.pending_rewards.checked_add(accrued).unwrap_or(i128::MAX); this keeps the read
path (pending_rewards) consistent with the checked write path used by stake,
unstake, and claim_rewards and references DataKey::Stake,
StakeInfo.pending_rewards, calculate_accrued_checked, and pending_rewards.
In `@frontend/src/components/NotificationCenter.tsx`:
- Around line 34-43: The Toast instance in NotificationCenter is being reused
across queued toasts causing stale internal state; update the JSX where
activeTipToast is rendered to supply a unique key prop (e.g., use
activeTipToast.id) on the Toast component so React mounts a fresh instance per
toast (look for activeTipToast and the Toast component render and the
dismissTipToast handler to confirm location).
In `@frontend/src/contexts/tipToastQueue.ts`:
- Around line 5-12: The TipToastQueueItem is missing the numeric amount so
insertByPriority() cannot sort within a priority tier; add an amount: number
field to the TipToastQueueItem interface and propagate notification.data.amount
into enqueueTipToast() (and any callers in useNotifications.ts) so that
insertByPriority() can compare amounts when ordering same-priority items; update
any places that construct TipToastQueueItem (including the enqueueTipToast call
sites) to include the amount and adjust insertByPriority() to use item.amount
for secondary sorting within the priority tier.
- Around line 51-54: getSnapshot currently returns a new object each call which
breaks useSyncExternalStore's referential stability; add a private cached
snapshot field (e.g., this._snapshot: TipToastQueueState) initialized to the
current state, make getSnapshot return that cached object, and update that
cached snapshot only inside emit() whenever this.active or this.queue changes
(and then call subscribers) so the snapshot reference remains stable between
actual state changes; reference getSnapshot, TipToastQueueState, emit,
this.active and this.queue when applying the change.
In `@frontend/src/hooks/useNotifications.ts`:
- Around line 35-44: The notification handling uses notification.id as the
canonical tip identifier, which allows duplicate tips if the envelope id
changes; update enqueueTipToastFromNotification (and other places that call
enqueueTipToast) to use notification.data.tipId for tipId and any seen-set/seed
keys, e.g., pass tipId: notification.data?.tipId ?? notification.id to
enqueueTipToast and seed the seen-set with the same value so tip replay dedupe
matches the backend's canonical id; ensure any fallback to notification.id is
explicit and documented and keep inferTipPriority(notification) usage unchanged.
---
Outside diff comments:
In `@contracts/contracts/staking/src/lib.rs`:
- Around line 110-149: The unstake function does not validate the input amount,
allowing negative or zero values to incorrectly increase balances and reset
cooldowns; add a guard at the start of pub fn unstake(env: Env, artist: Address,
amount: i128) to require amount > 0 (return Err(Error::InvalidAmount) or an
appropriate error) before artist.require_auth() or immediately after it, so
downstream mutations to StakeInfo.amount, TotalStaked, and UnstakeRequest
(DataKey::Unstake) cannot be performed with non-positive amounts and all
subsequent checked_add/checked_sub operations remain safe (affecting StakeInfo,
TotalStaked, and UnstakeRequest.amount, and use of COOLDOWN_LEDGERS).
---
Nitpick comments:
In `@contracts/contracts/staking/src/test.rs`:
- Around line 570-593: The test
test_slash_restore_invariants_do_not_corrupt_totals should assert the
sum-of-balances invariant: compute the per-account stakes (via c.get_stake(...)
or c.get_stake(&t.artistX).amount) for artist1 and artist2 and assert
artist1.amount + artist2.amount == c.total_staked() before calling
c.slash(&t.artist1), immediately after the slash (using the observed
slash_amount), and again after c.restore(&t.artist1) and the final
c.stake(&t.artist1, &MIN_STAKE) to ensure slash/restore didn't corrupt artist2
or TotalStaked; update the assertions in that test around the slash/restore
points accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa8ec1e0-875f-49c3-828e-9fefa8ae6475
📒 Files selected for processing (8)
contracts/contracts/staking/Cargo.tomlcontracts/contracts/staking/src/lib.rscontracts/contracts/staking/src/test.rsfrontend/src/components/NotificationCenter.tsxfrontend/src/components/Toast.tsxfrontend/src/contexts/tipToastQueue.test.tsfrontend/src/contexts/tipToastQueue.tsfrontend/src/hooks/useNotifications.ts
Comment on lines
285
to
+291
| pub fn pending_rewards(env: Env, artist: Address) -> i128 { | ||
| match env.storage().persistent().get::<DataKey, StakeInfo>(&DataKey::Stake(artist)) { | ||
| None => 0, | ||
| Some(i) => i.pending_rewards + Self::calculate_accrued(&env, &i), | ||
| Some(i) => i | ||
| .pending_rewards | ||
| .checked_add(Self::calculate_accrued(&env, &i)) | ||
| .unwrap_or(i128::MAX), |
There was a problem hiding this comment.
Clamp overflow before returning pending_rewards().
This unwrap_or(i128::MAX) only protects the final addition. Self::calculate_accrued(&env, &i) still maps calculate_accrued_checked() errors to 0 via Line 343, so the view can under-report the same position that stake, unstake, and claim_rewards now reject with Error::Overflow.
Keep the read path consistent with the checked write path
pub fn pending_rewards(env: Env, artist: Address) -> i128 {
match env.storage().persistent().get::<DataKey, StakeInfo>(&DataKey::Stake(artist)) {
None => 0,
- Some(i) => i
- .pending_rewards
- .checked_add(Self::calculate_accrued(&env, &i))
- .unwrap_or(i128::MAX),
+ Some(i) => Self::calculate_accrued_checked(&env, &i)
+ .ok()
+ .and_then(|accrued| i.pending_rewards.checked_add(accrued))
+ .unwrap_or(i128::MAX),
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn pending_rewards(env: Env, artist: Address) -> i128 { | |
| match env.storage().persistent().get::<DataKey, StakeInfo>(&DataKey::Stake(artist)) { | |
| None => 0, | |
| Some(i) => i.pending_rewards + Self::calculate_accrued(&env, &i), | |
| Some(i) => i | |
| .pending_rewards | |
| .checked_add(Self::calculate_accrued(&env, &i)) | |
| .unwrap_or(i128::MAX), | |
| pub fn pending_rewards(env: Env, artist: Address) -> i128 { | |
| match env.storage().persistent().get::<DataKey, StakeInfo>(&DataKey::Stake(artist)) { | |
| None => 0, | |
| Some(i) => Self::calculate_accrued_checked(&env, &i) | |
| .ok() | |
| .and_then(|accrued| i.pending_rewards.checked_add(accrued)) | |
| .unwrap_or(i128::MAX), | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/contracts/staking/src/lib.rs` around lines 285 - 291,
pending_rewards currently uses calculate_accrued (which hides overflow) and only
clamps the final addition; change it to call calculate_accrued_checked and if
that returns Err (overflow) return i128::MAX, otherwise perform pending =
i.pending_rewards.checked_add(accrued).unwrap_or(i128::MAX); this keeps the read
path (pending_rewards) consistent with the checked write path used by stake,
unstake, and claim_rewards and references DataKey::Stake,
StakeInfo.pending_rewards, calculate_accrued_checked, and pending_rewards.
Comment on lines
+34
to
+43
| {activeTipToast && ( | ||
| <Toast | ||
| id={activeTipToast.id} | ||
| type="tip" | ||
| title={activeTipToast.title} | ||
| message={activeTipToast.message} | ||
| duration={activeTipToast.duration} | ||
| priority={activeTipToast.priority} | ||
| onClose={dismissTipToast} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the NotificationCenter.tsx file and examine lines 34-43
fd -t f NotificationCenter.tsxRepository: OlufunbiIK/tip-tune
Length of output: 109
🏁 Script executed:
# Also find Toast.tsx
fd -t f Toast.tsxRepository: OlufunbiIK/tip-tune
Length of output: 96
🏁 Script executed:
# Read NotificationCenter.tsx to examine the current code around lines 34-43
cat -n frontend/src/components/NotificationCenter.tsxRepository: OlufunbiIK/tip-tune
Length of output: 5511
🏁 Script executed:
# Read Toast.tsx to understand its state management
cat -n frontend/src/components/Toast.tsxRepository: OlufunbiIK/tip-tune
Length of output: 5185
🏁 Script executed:
# Find and examine the tipToastQueue context to understand the queue flow
fd -t f tipToastQueueRepository: OlufunbiIK/tip-tune
Length of output: 145
🏁 Script executed:
# Examine the tipToastQueue context implementation
cat -n frontend/src/contexts/tipToastQueue.tsRepository: OlufunbiIK/tip-tune
Length of output: 4996
Add a key so each queued toast gets a fresh lifecycle.
Toast.tsx maintains timer and exit state internally. Without a key here, React reuses that component instance for the next active toast, so the next toast inherits the stale exiting state from its predecessor and renders with the exit animation immediately.
Suggested fix
{activeTipToast && (
<Toast
+ key={activeTipToast.id}
id={activeTipToast.id}
type="tip"
title={activeTipToast.title}
message={activeTipToast.message}
duration={activeTipToast.duration}
priority={activeTipToast.priority}
onClose={dismissTipToast}
/>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {activeTipToast && ( | |
| <Toast | |
| id={activeTipToast.id} | |
| type="tip" | |
| title={activeTipToast.title} | |
| message={activeTipToast.message} | |
| duration={activeTipToast.duration} | |
| priority={activeTipToast.priority} | |
| onClose={dismissTipToast} | |
| /> | |
| {activeTipToast && ( | |
| <Toast | |
| key={activeTipToast.id} | |
| id={activeTipToast.id} | |
| type="tip" | |
| title={activeTipToast.title} | |
| message={activeTipToast.message} | |
| duration={activeTipToast.duration} | |
| priority={activeTipToast.priority} | |
| onClose={dismissTipToast} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/NotificationCenter.tsx` around lines 34 - 43, The
Toast instance in NotificationCenter is being reused across queued toasts
causing stale internal state; update the JSX where activeTipToast is rendered to
supply a unique key prop (e.g., use activeTipToast.id) on the Toast component so
React mounts a fresh instance per toast (look for activeTipToast and the Toast
component render and the dismissTipToast handler to confirm location).
Comment on lines
+5
to
+12
| export interface TipToastQueueItem { | ||
| id: string; | ||
| tipId: string; | ||
| title: string; | ||
| message: string; | ||
| priority: ToastPriority; | ||
| createdAt: string; | ||
| duration?: number; |
There was a problem hiding this comment.
The queue still loses actual amount ordering inside each priority tier.
By the time an item reaches insertByPriority(), the numeric amount is gone. That means two "high" tips are still FIFO, so a 30 XLM tip can stay ahead of a later 100 XLM tip even though the requirement is to rank by tip magnitude.
Suggested fix
export interface TipToastQueueItem {
id: string;
tipId: string;
title: string;
message: string;
+ amount: number;
priority: ToastPriority;
createdAt: string;
duration?: number;
}
@@
interface EnqueueTipToastInput {
id: string;
tipId: string;
title: string;
message: string;
+ amount: number;
priority: ToastPriority;
createdAt?: string;
duration?: number;
}
@@
private insertByPriority(item: StoredQueueItem): void {
this.queue.push(item);
this.queue.sort((left, right) => {
if (left.priority !== right.priority) {
return left.priority === 'high' ? -1 : 1;
}
+ if (left.amount !== right.amount) {
+ return right.amount - left.amount;
+ }
return left.sequence - right.sequence;
});
}You'll also need to thread notification.data.amount into enqueueTipToast() from frontend/src/hooks/useNotifications.ts.
Also applies to: 20-28, 120-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/contexts/tipToastQueue.ts` around lines 5 - 12, The
TipToastQueueItem is missing the numeric amount so insertByPriority() cannot
sort within a priority tier; add an amount: number field to the
TipToastQueueItem interface and propagate notification.data.amount into
enqueueTipToast() (and any callers in useNotifications.ts) so that
insertByPriority() can compare amounts when ordering same-priority items; update
any places that construct TipToastQueueItem (including the enqueueTipToast call
sites) to include the amount and adjust insertByPriority() to use item.amount
for secondary sorting within the priority tier.
Comment on lines
+51
to
+54
| getSnapshot = (): TipToastQueueState => ({ | ||
| active: this.active, | ||
| queuedCount: this.queue.length, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "tipToastQueue.ts" -type fRepository: OlufunbiIK/tip-tune
Length of output: 103
🏁 Script executed:
cat -n frontend/src/contexts/tipToastQueue.tsRepository: OlufunbiIK/tip-tune
Length of output: 4996
🏁 Script executed:
rg "useSyncExternalStore" --type ts --type tsx -B 3 -A 3Repository: OlufunbiIK/tip-tune
Length of output: 90
getSnapshot() must return a stable object reference to satisfy useSyncExternalStore contract.
React 18's useSyncExternalStore requires that the snapshot returned by getSnapshot() maintains referential equality (same object reference) until the underlying store changes. Currently, returning a fresh object literal on every call causes React to perceive state changes that haven't occurred, triggering unnecessary re-renders and potentially "Uncached snapshot" warnings in Strict Mode.
Cache the snapshot as a private field and update it only within emit():
Suggested fix
class TipToastQueueStore {
+ private snapshot: TipToastQueueState = {
+ active: null,
+ queuedCount: 0,
+ };
private queue: StoredQueueItem[] = [];
private active: StoredQueueItem | null = null;
@@
- getSnapshot = (): TipToastQueueState => ({
- active: this.active,
- queuedCount: this.queue.length,
- });
+ getSnapshot = (): TipToastQueueState => this.snapshot;
@@
private emit(): void {
+ this.snapshot = {
+ active: this.active,
+ queuedCount: this.queue.length,
+ };
this.listeners.forEach((listener) => listener());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| getSnapshot = (): TipToastQueueState => ({ | |
| active: this.active, | |
| queuedCount: this.queue.length, | |
| }); | |
| class TipToastQueueStore { | |
| private snapshot: TipToastQueueState = { | |
| active: null, | |
| queuedCount: 0, | |
| }; | |
| private queue: StoredQueueItem[] = []; | |
| private active: StoredQueueItem | null = null; | |
| getSnapshot = (): TipToastQueueState => this.snapshot; | |
| private emit(): void { | |
| this.snapshot = { | |
| active: this.active, | |
| queuedCount: this.queue.length, | |
| }; | |
| this.listeners.forEach((listener) => listener()); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/contexts/tipToastQueue.ts` around lines 51 - 54, getSnapshot
currently returns a new object each call which breaks useSyncExternalStore's
referential stability; add a private cached snapshot field (e.g.,
this._snapshot: TipToastQueueState) initialized to the current state, make
getSnapshot return that cached object, and update that cached snapshot only
inside emit() whenever this.active or this.queue changes (and then call
subscribers) so the snapshot reference remains stable between actual state
changes; reference getSnapshot, TipToastQueueState, emit, this.active and
this.queue when applying the change.
Comment on lines
+35
to
+44
| const enqueueTipToastFromNotification = useCallback((notification: Notification) => { | ||
| enqueueTipToast({ | ||
| id: notification.id, | ||
| tipId: notification.id, | ||
| title: notification.title, | ||
| message: notification.message, | ||
| priority: inferTipPriority(notification), | ||
| createdAt: notification.createdAt, | ||
| duration: 5000, | ||
| }); |
There was a problem hiding this comment.
Use data.tipId consistently for tip replay suppression.
The backend exposes the canonical tip id as notification.data.tipId (backend/src/tips/tips.service.ts:209-213, backend/src/websocket/websocket.gateway.ts:17-29). Keying both the seen-set seed and enqueueTipToast() off notification.id means the same tip can still duplicate the dropdown/unread count and bypass queue dedupe if it is replayed with a different notification envelope.
Suggested fix
- const seenNotificationIdsRef = useRef<Set<string>>(new Set());
+ const seenTipEventIdsRef = useRef<Set<string>>(new Set());
+
+ const getTipEventId = (notification: Notification) =>
+ notification.data?.tipId ?? notification.id;
const enqueueTipToastFromNotification = useCallback((notification: Notification) => {
enqueueTipToast({
id: notification.id,
- tipId: notification.id,
+ tipId: getTipEventId(notification),
title: notification.title,
message: notification.message,
priority: inferTipPriority(notification),
createdAt: notification.createdAt,
duration: 5000,
@@
- seenNotificationIdsRef.current = new Set(
+ seenTipEventIdsRef.current = new Set(
(response.data.data as Notification[])
- .map((notification) => notification.id)
+ .map(getTipEventId)
.filter((id): id is string => Boolean(id)),
);
@@
- if (seenNotificationIdsRef.current.has(notification.id)) {
+ const tipEventId = getTipEventId(notification);
+ if (seenTipEventIdsRef.current.has(tipEventId)) {
return;
}
- seenNotificationIdsRef.current.add(notification.id);
+ seenTipEventIdsRef.current.add(tipEventId);Also applies to: 52-56, 84-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/hooks/useNotifications.ts` around lines 35 - 44, The
notification handling uses notification.id as the canonical tip identifier,
which allows duplicate tips if the envelope id changes; update
enqueueTipToastFromNotification (and other places that call enqueueTipToast) to
use notification.data.tipId for tipId and any seen-set/seed keys, e.g., pass
tipId: notification.data?.tipId ?? notification.id to enqueueTipToast and seed
the seen-set with the same value so tip replay dedupe matches the backend's
canonical id; ensure any fallback to notification.id is explicit and documented
and keep inferTipPriority(notification) usage unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[Frontend] Real-Time Tip Toasts with Priority Queue (Closes #246)
Overview
This PR addresses an issue where concurrent live tip events could overlap or be missed by the UI. I have implemented a robust toast notification system driven by a Priority Queue to ensure high-value interactions are highlighted first and duplicate events are suppressed during high-traffic periods.
Feature Summary
tipIdtracking mechanism to prevent duplicate toast triggers from redundant socket events.Technical Implementation
frontend/src/contextsthat manages an ordered array based on tip magnitude.Setto track processedtipIds within the current session window, filtering out any replays.frontend/src/components/tipto subscribe to the new priority state, using CSS transitions to handle the entrance/exit animations as the queue drains.Test Coverage
Checklists
[Soroban] Staking Reward Precision, Cooldown Logic, and Slashing Invariants (Closes #319)
Overview
This PR strengthens the core accounting logic of the Staking contract. It focuses on ensuring high-precision reward accrual, robust cooldown/withdrawal lifecycle management, and the preservation of system-wide invariants during punitive actions like slashing and restoration.
Feature Summary
Technical Implementation
total_stakedalways matches the sum of individual balances, even across slashing transitions.unstake -> cooldown -> withdrawflow to prevent premature or double-withdrawals.StakingEventcoverage to include granular metadata for reward claims and administrative adjustments.Test Coverage
Checklists
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests